Skip to content

Conversation

jordan-powers
Copy link
Contributor

Follow-up to #133839 to remove the feature flag and enable the feature in production.

Keeping it a draft because a couple of the graphs on the nightlies look concerning (limit_500 and chicken_1).

@elasticsearchmachine
Copy link
Collaborator

Hi @jordan-powers, I've created a changelog YAML for you.

@martijnvg
Copy link
Member

Would you be able to get jfr / flame graphs from a a run before and after the increase for limit_500? The limit 500 returns all fields. Before all unmapped fields were in one ignored source stored field entry whereas now it is in multiple. This could be a reason in the difference.

@jordan-powers
Copy link
Contributor Author

After analyzing the flamegraphs, I think the regressions are due to the newly-added runtime fields in elastic/rally-tracks#841.

The regressions are due to a change in the benchmark and are unrelated to the new field visitor.

In the Sep. 8 flamegraphs, we see time spent in the KeywordScriptDocValuesReader and the LongScriptDocValuesReader. These classes are not present in the Sept. 4 flamegraphs.

image

Flamegraphs:

@jordan-powers jordan-powers marked this pull request as ready for review September 23, 2025 17:49
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

@elasticsearchmachine
Copy link
Collaborator

Hi @jordan-powers, I've updated the changelog YAML for you.

@martijnvg
Copy link
Member

Ok, I see, so the addition of runtime fields in the mapping caused these fields also to be retrieved by the limit_500 esql query because it fetches all fields including runtime fields. This makes sense, thanks for checking 👍

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@elasticsearchmachine elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Sep 25, 2025
@jordan-powers jordan-powers removed the serverless-linked Added by automation, don't add manually label Sep 25, 2025
@jordan-powers
Copy link
Contributor Author

It's too much trouble to rename the node feature. If we decide we really need to rename it, it can happen in a follow-up, with guidance from core-infra.

@jordan-powers jordan-powers enabled auto-merge (squash) September 26, 2025 15:28
@jordan-powers jordan-powers merged commit 322e5fe into elastic:main Sep 26, 2025
34 checks passed
@jordan-powers jordan-powers deleted the ignored-source-coalesce-entries-remove-ff branch October 1, 2025 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants